Addressed review in #1247 by VisLab · Pull Request #1263 · hed-standard/hed-python

VisLab · 2026-03-11T22:06:32Z

No description provided.

github-actions · 2026-03-11T22:09:30Z

hed/schema/schema_io/base2schema.py

            if not schema.with_standard:
                raise HedFileError(
-                    HedExceptions.ROOTED_TAG_INVALID,
+                    HedExceptions.SCHEMA_ATTRIBUTE_INVALID,


Critical: AttributeError at runtime — HedExceptions.SCHEMA_ATTRIBUTE_INVALID does not exist on HedExceptions. That constant lives on SchemaAttributeErrors in hed/errors/error_types.py.

Either:

Change to HedExceptions.SCHEMA_LIBRARY_INVALID (consistent with the other rooted-tag errors below), or

Import SchemaAttributeErrors from hed.errors.error_types and use SchemaAttributeErrors.SCHEMA_ATTRIBUTE_INVALID.

Suggested change

HedExceptions.SCHEMA_ATTRIBUTE_INVALID,

HedExceptions.SCHEMA_LIBRARY_INVALID,

github-actions · 2026-03-11T22:09:31Z

hed/schema/schema_io/base2schema.py

            if not rooted_entry or rooted_entry.has_attribute(constants.HedKey.InLibrary):
                raise HedFileError(
-                    HedExceptions.ROOTED_TAG_DOES_NOT_EXIST,
+                    HedExceptions.LIBRARY_SCHEMA_INVALID,


Critical: AttributeError at runtime — HedExceptions.LIBRARY_SCHEMA_INVALID does not exist. The constant is SCHEMA_LIBRARY_INVALID (note the word order).

Suggested change

HedExceptions.LIBRARY_SCHEMA_INVALID,

HedExceptions.SCHEMA_LIBRARY_INVALID,

github-actions · 2026-03-11T22:09:42Z

hed/errors/exceptions.py


    # These are actual schema issues, not that the file cannot be found or parsed
-    SCHEMA_HEADER_MISSING = "SCHEMA_HEADER_INVALID"
    SCHEMA_HEADER_INVALID = "SCHEMA_HEADER_INVALID"


Critical: Incomplete removal — AttributeError at runtime in three files

IN_LIBRARY_IN_UNMERGED and INVALID_LIBRARY_PREFIX were removed from HedExceptions, but they are still referenced in files not touched by this PR:

File Line Removed constant

hed/schema/schema_io/xml2schema.py 351 HedExceptions.IN_LIBRARY_IN_UNMERGED

hed/schema/schema_io/df2schema.py 278 HedExceptions.IN_LIBRARY_IN_UNMERGED

hed/schema/schema_io/wiki2schema.py 612 HedExceptions.IN_LIBRARY_IN_UNMERGED

hed/schema/hed_schema.py 426 HedExceptions.INVALID_LIBRARY_PREFIX

Each of those call sites needs to be updated to its replacement code (SCHEMA_LIBRARY_INVALID for IN_LIBRARY_IN_UNMERGED, and SCHEMA_LIBRARY_INVALID for INVALID_LIBRARY_PREFIX) before merging.

github-actions · 2026-03-11T22:09:57Z

Review Summary

This PR consolidates redundant HedExceptions aliases into their canonical forms and fixes several real bugs. The cleanups are good, but the alias removal is incomplete and introduces four new AttributeError crash paths.

Critical — will crash at runtime

1. HedExceptions.SCHEMA_ATTRIBUTE_INVALID doesn't exist (base2schema.py:197)
SCHEMA_ATTRIBUTE_INVALID lives on SchemaAttributeErrors (in error_types.py), not on HedExceptions. Replace with HedExceptions.SCHEMA_LIBRARY_INVALID or import SchemaAttributeErrors and use the correct class.

2. HedExceptions.LIBRARY_SCHEMA_INVALID doesn't exist (base2schema.py:226)
The constant is SCHEMA_LIBRARY_INVALID (word order is reversed). One-character rename.

3. IN_LIBRARY_IN_UNMERGED removed but still used in three files
The constant was deleted from HedExceptions, but these call sites were not updated:

hed/schema/schema_io/xml2schema.py:351
hed/schema/schema_io/df2schema.py:278
hed/schema/schema_io/wiki2schema.py:612

All three need HedExceptions.IN_LIBRARY_IN_UNMERGED → HedExceptions.SCHEMA_LIBRARY_INVALID.

4. INVALID_LIBRARY_PREFIX removed but still used
Deleted from HedExceptions, but hed/schema/hed_schema.py:426 still references it.
Replace with HedExceptions.SCHEMA_LIBRARY_INVALID.

Confirmed good fixes in this PR

ONSET_TOLERANCE = 10 - 7 (evaluates to 3) → 1e-7 — correct scientific-notation literal
self.tag_dict[tag].value_dict + val_count → self.tag_dict[tag].value_dict[value] + val_count — correct subscript in dict merge
Missing closing ' in two val_error_def_* messages
Stray } in bids_dataset.py error string
Regex compiled to module-level COLUMN_REF_PATTERN constant
Removal of make_path (function had no return statement, always returned None)
Dead commented-out code removed

The alias consolidation approach is correct; it just needs the four call sites above updated before merging.

Copilot

Pull request overview

This PR appears to follow up on review feedback from #1247 by standardizing schema-related error codes/messages, cleaning up dead/commented code, and making a few small validator/analysis fixes.

Changes:

Consolidates multiple schema/header/library error codes into fewer canonical HedExceptions codes and updates tests accordingly.
Refactors/cleans up validators and tools (e.g., precompiled column-ref regex, removes unused make_path, removes commented code).
Fixes a few correctness issues in analysis/utilities (e.g., tag count merge bug, onset tolerance constant, error message typos).

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/schema/test_schema_wiki_fatal_errors.py	Updates expected fatal error codes for wiki schema header cases.
tests/schema/test_hed_schema_io.py	Updates prerelease-partner tests/docs to assert the new error code.
spec_tests/test_hed_cache.py	Updates expected exception code for invalid merged schema load cases.
hed/validator/util/group_util.py	Removes unused commented-out validator hooks.
hed/validator/spreadsheet_validator.py	Fixes onset tolerance constant to a small float tolerance.
hed/validator/sidecar_validator.py	Precompiles the column reference regex and reuses it.
hed/validator/def_validator.py	Removes commented-out unit validation code.
hed/tools/util/io_util.py	Removes the unused `make_path` helper.
hed/tools/bids/bids_dataset.py	Fixes an error message typo (extra `}`).
hed/tools/analysis/hed_type_factors.py	Normalizes factor encoding errors to raise `HedFileError` and fixes a doc typo.
hed/tools/analysis/hed_tag_counts.py	Fixes merge logic when summing per-value counts.
hed/tools/analysis/annotation_util.py	Removes large blocks of commented-out legacy helpers.
hed/tools/init.py	Stops exporting `make_path` (aligned with its removal).
hed/schema/schema_io/wiki2schema.py	Uses `SCHEMA_HEADER_INVALID` for missing/invalid header line.
hed/schema/schema_io/base2schema.py	Adjusts error codes for schema merge/load and rooted-tag validation paths.
hed/schema/schema_header_util.py	Improves library-name validation messages and aligns error codes.
hed/schema/hed_schema_group.py	Uses `SCHEMA_LOAD_FAILED` for duplicate namespace/prefix collisions.
hed/errors/exceptions.py	Removes legacy/alias exception constants in favor of canonical codes.
hed/errors/error_messages.py	Fixes missing trailing quote in formatted def/def-expand messages.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T22:09:57Z

hed/schema/schema_io/base2schema.py

            if not schema.with_standard:
                raise HedFileError(
-                    HedExceptions.ROOTED_TAG_INVALID,
+                    HedExceptions.SCHEMA_ATTRIBUTE_INVALID,


HedExceptions.SCHEMA_ATTRIBUTE_INVALID is referenced here, but HedExceptions (hed/errors/exceptions.py) does not define this constant. This will raise an AttributeError at runtime when a rooted tag is encountered. Either add SCHEMA_ATTRIBUTE_INVALID to HedExceptions or use an existing HedExceptions.* code that matches this condition (or switch this path to use the schema attribute error code source consistently).

Suggested change

HedExceptions.SCHEMA_ATTRIBUTE_INVALID,

HedExceptions.SCHEMA_LIBRARY_INVALID,

Copilot · 2026-03-11T22:09:58Z

hed/schema/schema_io/base2schema.py

                raise HedFileError(
-                    HedExceptions.ROOTED_TAG_INVALID,
+                    HedExceptions.SCHEMA_LIBRARY_INVALID,
                    f"Rooted tag '{tag_entry.short_tag_name}' is not a string.\"",


The error message string ends with an extra escaped quote (... is not a string."), which will produce a confusing message (trailing "). Remove the stray quote so the message is clean.

Suggested change

f"Rooted tag '{tag_entry.short_tag_name}' is not a string.\"",

f"Rooted tag '{tag_entry.short_tag_name}' is not a string.",

Copilot · 2026-03-11T22:09:58Z

hed/schema/schema_io/base2schema.py

            if not rooted_entry or rooted_entry.has_attribute(constants.HedKey.InLibrary):
                raise HedFileError(
-                    HedExceptions.ROOTED_TAG_DOES_NOT_EXIST,
+                    HedExceptions.LIBRARY_SCHEMA_INVALID,


HedExceptions.LIBRARY_SCHEMA_INVALID is referenced here, but HedExceptions (hed/errors/exceptions.py) does not define this constant. This will raise an AttributeError at runtime. Use an existing error code (e.g., SCHEMA_LIBRARY_INVALID if appropriate) or define LIBRARY_SCHEMA_INVALID in HedExceptions.

Suggested change

HedExceptions.LIBRARY_SCHEMA_INVALID,

HedExceptions.SCHEMA_LIBRARY_INVALID,

Addressed review in hed-standard#1247

1d8a15e

VisLab requested a review from Copilot March 11, 2026 22:06

Copilot started reviewing on behalf of VisLab March 11, 2026 22:07 View session

github-actions bot reviewed Mar 11, 2026

View reviewed changes

Copilot AI reviewed Mar 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addressed review in #1247#1263

Addressed review in #1247#1263
VisLab wants to merge 1 commit intohed-standard:mainfrom
VisLab:fix_extras

VisLab commented Mar 11, 2026

Uh oh!

github-actions bot Mar 11, 2026

Uh oh!

github-actions bot Mar 11, 2026

Uh oh!

github-actions bot Mar 11, 2026

Uh oh!

github-actions bot commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	HedExceptions.SCHEMA_ATTRIBUTE_INVALID,
	HedExceptions.SCHEMA_LIBRARY_INVALID,

	HedExceptions.LIBRARY_SCHEMA_INVALID,
	HedExceptions.SCHEMA_LIBRARY_INVALID,

File	Line	Removed constant
`hed/schema/schema_io/xml2schema.py`	351	`HedExceptions.IN_LIBRARY_IN_UNMERGED`
`hed/schema/schema_io/df2schema.py`	278	`HedExceptions.IN_LIBRARY_IN_UNMERGED`
`hed/schema/schema_io/wiki2schema.py`	612	`HedExceptions.IN_LIBRARY_IN_UNMERGED`
`hed/schema/hed_schema.py`	426	`HedExceptions.INVALID_LIBRARY_PREFIX`

	f"Rooted tag '{tag_entry.short_tag_name}' is not a string.\"",
	f"Rooted tag '{tag_entry.short_tag_name}' is not a string.",

Conversation

VisLab commented Mar 11, 2026

Uh oh!

github-actions bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 11, 2026

Review Summary

Critical — will crash at runtime

Confirmed good fixes in this PR

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants